Skip to content

feat(filter): Add Slider Range Inputs Option for Numerical Range Filters #33170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

feat(filter): Add Slider Range Inputs Option for Numerical Range Filters #33170

wants to merge 21 commits into from

Conversation

payose
Copy link

@payose payose commented Apr 17, 2025 •
edited
Loading

SUMMARY
This PR reintroduces the slider as an additional method for configuring numerical range filters, based on community feedback following its removal in #31726.

The main improvements include:

  • Allowing users to select between Slider, Range Inputs, or both when configuring a numerical range filter.
  • Supporting cases where only one of the min or max values is configured, removing the previous dependency on both values.
  • Enhancing usability for large ranges by making the slider available.
  • Improving user guidance with placeholder defaults and metadata:
  • Placeholders display the dataset’s min and max values.
  • Metadata text like "Choose numbers between [x] and [x]" shown under the filter bar and in tooltips.
  • Handling synchronization between slider and inputs: Updating inputs dynamically when the slider changes, and vice versa.
  • UI behavior differs based on orientation:
    Horizontal mode: Slider rendered first, then inputs inline.
    Vertical mode: Metadata and error states positioned appropriately.
  • Updated error messages to include the actual valid ranges.
  • Ensured compatibility with the existing "Single value" filter configuration.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
image
image

image
image

image
image

TESTING INSTRUCTIONS
Open a dashboard and add or edit a numerical range filter.

In filter settings, configure:

  • Only slider enabled.
  • Only range inputs enabled.
  • Both slider and range inputs enabled.

In vertical orientation:

  • Confirm metadata text appears correctly.
  • Confirm error states are shown under the filter with correct ranges.

In horizontal orientation:

  • Confirm slider is shown first, inputs next to it.
  • Error is displayed via tooltip
  • Tooltip turns red to indicate when an error occurs

Test synchronization:

  • Verify placeholders in inputs show dataset default min/max.
  • Move the slider and verify input values update accordingly.
  • Change input values and verify slider updates accordingly.
  • Leave min or max empty intentionally and verify filter works with just one value.
  • Validate that "Single value" mode continues working without issues.

ADDITIONAL INFORMATION
Has associated issue: #33150

@korbit-ai Korbit AI
Copy link

korbit-ai bot commented Apr 17, 2025

Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment /korbit-review.

Your admin can change your review schedule in the Korbit Console

@payose payose changed the title Re-implement Slider in Numerical Range filter feat(filter): Add Slider Range Inputs Option for Numerical Range Filters Apr 17, 2025
@payose payose marked this pull request as ready for review April 23, 2025 17:01
@dosubot dosubot bot added the dashboard:native-filters Related to the native filters of the Dashboard label Apr 23, 2025
korbit-ai[bot]
korbit-ai bot reviewed Apr 23, 2025
View reviewed changes
Copy link

@korbit-ai korbit-ai bot left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Design Redundant Data Structure ▹ view ✅ Fix detected
Readability Complex Validation Logic ▹ view ✅ Fix detected
Functionality Unreliable Modal Save Logic ▹ view ✅ Fix detected
Error Handling Silent failure in input handler ▹ view ✅ Fix detected
Readability Replace sequential waits with clear state expectations ▹ view ✅ Fix detected
Files scanned ​
File Path Reviewed
superset-frontend/src/filters/components/common.ts ✅
superset-frontend/src/filters/components/Range/types.ts ✅
superset-frontend/src/dashboard/components/nativeFilters/utils.ts ✅
superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts ✅
superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx ✅
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx ✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

@geido geido added hold! On hold preset:bounty Issues that have been selected by Preset and have a bounty attached. testenv-up labels Apr 24, 2025
@github-actions GitHub Actions
Copy link
Contributor

@geido Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

@github-actions GitHub Actions
Copy link
Contributor

@geido Ephemeral environment spinning up at http://54.213.24.145:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

@geido
Copy link
Member

geido commented Apr 24, 2025

Just a typo. There is a semicolon in the text.

Screenshot 2025-04-24 at 18 02 45

There is a more serious bug that looks weird. When changing the type of filter, for example going from Slider + Inputs to just Slider and then applying the filter on the Dashboard, the charts affected by the filter won't show the filter applied as they normally do.

Screenshot 2025-04-24 at 18 13 55

Updating the input with both up/down arrows or manually does not reflect on the slider

Screenshot 2025-04-24 at 18 17 07

I have selected the option "Single value" in the filter settings and the UI breaks. Also, it seems that the inputs are not respecting the single value any longer and are still showing both min and max inputs instead of just one as expected.

Screenshot 2025-04-24 at 18 18 36

It looks like there are still relevant issues in the PR so I am not pausing testing here to give you a chance to fix the major problems first.

Thank you

@mistercrunch
Copy link
Member

@kasiazjc

@sadpandajoe
Copy link
Member

@Vitor-Avila I think this was one of the changes we talked about to make 5.0 parity with what our existing customers have

msyavuz
msyavuz reviewed May 2, 2025
View reviewed changes
Copy link
Member

@msyavuz msyavuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, I don't see any other issues from testing. There are a couple of weird behaviors that exist on master as well, though I don't think they are in the scope of this PR.

@payose payose force-pushed the input-plus-slider-range branch from dcd706d to de6a501 Compare May 5, 2025 14:05
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 5, 2025

@sadpandajoe Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 5, 2025

@sadpandajoe Ephemeral environment spinning up at http://34.216.248.51:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

@sadpandajoe
Copy link
Member

One issue I found with this latest update is that if I hit the down arrow on the max range field, it goes to -1 instead of max -1

Screen.Recording.2025-05-05.at.10.36.36.PM.mov

@payose
Copy link
Author

payose commented May 5, 2025

One issue I found with this latest update is that if I hit the down arrow on the max range field, it goes to -1 instead of max -1

Screen.Recording.2025-05-05.at.10.36.36.PM.mov

Thanks for pointing this out. It has now been fixed.

msyavuz
msyavuz reviewed May 6, 2025
View reviewed changes
Copy link
Member

@msyavuz msyavuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is an issue with default filters when i type the min value and move on to the max it types the max immediatly after a character:
https://github.com/user-attachments/assets/1599f6da-c9ec-400a-9647-7062cf977e78

@msyavuz
Copy link
Member

msyavuz commented May 7, 2025

Hey, here are some other things I noticed from manual testing:

  1. When adding a filter and setting min value, if the max value is not set and Single Value filter option is not enabled, should the filter be valid? This was not the case before, i don't think this counts as a regression, but it's definitely a behavior change.

  2. Placeholder for the input and slider is different when exact single value is selected
    image

  3. If filter value is required, users should not be able to clear all and apply.

@payose
Copy link
Author

payose commented May 8, 2025 •
edited
Loading

#33150

No. 1 is an update described in the issue for this PR. The empty field will default to the respective min or max value.

Remove the need of having both values configured. Allow to keep one of the two empty.

other issues have fixed in the last commit

@github-actions GitHub Actions
Copy link
Contributor

@geido Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

@github-actions GitHub Actions
Copy link
Contributor

@geido Ephemeral environment spinning up at http://44.248.62.134:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geido geido geido left review comments

@korbit-ai korbit-ai[bot] korbit-ai[bot] left review comments

@msyavuz msyavuz msyavuz approved these changes

@sadpandajoe sadpandajoe Awaiting requested review from sadpandajoe sadpandajoe is a code owner

@eschutho eschutho Awaiting requested review from eschutho eschutho is a code owner

@rusackas rusackas Awaiting requested review from rusackas rusackas is a code owner

@betodealmeida betodealmeida Awaiting requested review from betodealmeida betodealmeida is a code owner

@mistercrunch mistercrunch Awaiting requested review from mistercrunch mistercrunch is a code owner

@michael-s-molina michael-s-molina Awaiting requested review from michael-s-molina michael-s-molina is a code owner

@kgabryje kgabryje Awaiting requested review from kgabryje kgabryje is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
dashboard:native-filters Related to the native filters of the Dashboard hold! On hold preset:bounty Issues that have been selected by Preset and have a bounty attached. size/XL testenv-up
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@payose @geido @mistercrunch @sadpandajoe @msyavuz @rusackas @michael-s-molina